Skip to content

Add server.request.body.files_content AppSec address for Akka HTTP#11725

Open
jandro996 wants to merge 5 commits into
masterfrom
filecontent-akka
Open

Add server.request.body.files_content AppSec address for Akka HTTP#11725
jandro996 wants to merge 5 commits into
masterfrom
filecontent-akka

Conversation

@jandro996

@jandro996 jandro996 commented Jun 24, 2026

Copy link
Copy Markdown
Member

What Does This Do

Extends the Akka HTTP AppSec instrumentation to dispatch the `server.request.body.files_content` WAF address for multipart file uploads.

  • Adds `contentCallback` (`EVENTS.requestFilesContent()`) to both multipart parsing routes in `UnmarshallerHelpers`:
    • `handleMultipartStrictFormData` — used by the `entity(as[Multipart.FormData.Strict])` / strict unmarshaller variant
    • `handleStrictFormData` — used by the `formFieldMultiMap` / `entity(as[StrictForm])` default route
  • Per-part file content is read via `ByteString.getData().take(MAX_CONTENT_BYTES).toArray()` to avoid allocating the full file in memory
  • Content is decoded using `MultipartContentDecoder.decodeBytes()` (introduced in Fix charset decoding for server.request.body.files_content in commons-fileupload #11212), with the correct `ContentType` API per route:
    • Route 1 (Java API): `entity.getContentType().toString()`
    • Route 2 (Scala API): `sentity.contentType().toString()`
  • `MAX_CONTENT_BYTES` and `MAX_FILES_TO_INSPECT` read from `Config.get()` (not hardcoded)
  • Dispatch follows the established sequential ordering: body and filenames always fire; `files_content` fires only if neither prior callback blocked the request
  • Hoists `reqCtx` in `handleStrictFormData` so it is shared by both the filenames and content dispatch blocks
  • Skips body decoding in `handleStrictFormData` when body callback is absent (avoids full-file string allocation for content-only subscribers)
  • Adds `testBodyFilesContent()` overrides in `akka-http-10.0` and `akka-http-10.6` test modules: `true` for async/bind variants, `false` for sync variants that do not process body

Motivation

Part of APPSEC-61875: extending `server.request.body.files_content` WAF address support across Java frameworks. Akka HTTP already has `server.request.body.filenames` support since #11173; this PR adds the content counterpart following the same pattern established in #11198 (Tomcat + Netty) and #11229 (Jersey + RESTEasy).

Additional Notes

`akka-http-10.6` tests require an Akka commercial repository token (`akkaRepositoryToken`) which is only available in CI. Local test runs cover `akka-http-10.0` only.

Contributor Checklist

Jira ticket: APPSEC-61875

Note: Once your PR is ready to merge, add it to the merge queue by commenting `/merge`. `/merge -c` cancels the queue request. `/merge -f --reason "reason"` skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.

…HTTP

- Extend handleMultipartStrictFormData (strictUnmarshaller) and
  handleStrictFormData (formFieldMultiMap) in UnmarshallerHelpers to
  accumulate file content via MultipartContentDecoder and dispatch
  EVENTS.requestFilesContent() callback
- Content dispatch is sequential after filenames (fires only if no
  prior block), consistent with other frameworks (Tomcat, Netty, Jersey)
- Uses ByteString.take(MAX_CONTENT_BYTES).toArray() to avoid full
  allocation; MAX_* constants read from Config
- Add testBodyFilesContent() overrides to both akka-http-10.0 and
  akka-http-10.6 test modules
@jandro996

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ace3f1abca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jandro996 jandro996 added type: enhancement Enhancements and improvements comp: asm waf Application Security Management (WAF) labels Jun 24, 2026
…lback absent

When only files_content is subscribed (not requestBodyProcessed),
the previous code still called getData().decodeString() on every file
body part to populate conv — decoding the full file into a String
regardless of MAX_CONTENT_BYTES. Gate decodeString and
handleArbitraryPostData on bodyCb != null to avoid unnecessary
full-file allocation.
…BindSyncTest

Sync server binding does not support body processing; override returns false
to match the pattern already used for testBodyMultipart, testBodyFilenames, etc.
…BindSyncTest (10.6)

Same fix as 10.0: sync binding does not process body, override returns false
to match testBodyMultipart, testBodyFilenames, etc.
@jandro996 jandro996 marked this pull request as ready for review June 24, 2026 13:13
@jandro996 jandro996 requested review from a team as code owners June 24, 2026 13:13
@jandro996 jandro996 requested review from ValentinZakharov, claponcet and manuel-alvarez-alvarez and removed request for a team June 24, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: asm waf Application Security Management (WAF) type: enhancement Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants